-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[DTLTO][TEST] Make Clang driver tests more robust #159151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DTLTO][TEST] Make Clang driver tests more robust #159151
Conversation
The Clang driver tests tried to match the Clang executable name via a regular expression. This failed on some buildbots where the name was not anticipated. Make the test more robust by extracting the filename with Python and appending a line to the output. This is then captured into a FileCheck variable and used directly in the check for the executable name. Should fix buildbot failures caused by the merge of PR llvm#159129.
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: bd1976bris (bd1976bris) ChangesThe Clang driver tests tried to match the Clang executable name via a regular expression. This failed on some buildbots where the name was not anticipated. Make the test more robust by extracting the filename with Python and appending a line to the output. This is then captured into a FileCheck variable and used directly in the check for the executable name. Should fix buildbot failures caused by the merge of PR #159129. Full diff: https://github.com/llvm/llvm-project/pull/159151.diff 3 Files Affected:
diff --git a/clang/test/Driver/DTLTO/dtlto.c b/clang/test/Driver/DTLTO/dtlto.c
index 053955a9e3d4a..8f086584e6ba7 100644
--- a/clang/test/Driver/DTLTO/dtlto.c
+++ b/clang/test/Driver/DTLTO/dtlto.c
@@ -6,14 +6,16 @@
/// Check DTLTO options are forwarded to the linker.
/// Check that options are forwarded as expected with --thinlto-distributor=.
+// RUN: %python %S/filename.py %clang > %t_forward.log
// RUN: %clang -flto=thin %s -### -fuse-ld=lld --target=x86_64-linux-gnu \
// RUN: -Xthinlto-distributor=a1 -Xthinlto-distributor=a2,a3 \
-// RUN: -fthinlto-distributor=d.exe -Werror 2>&1 | \
-// RUN: FileCheck %s --check-prefix=FORWARD
+// RUN: -fthinlto-distributor=d.exe -Werror >>%t_forward.log 2>&1
+// RUN: FileCheck %s --input-file=%t_forward.log --check-prefix=FORWARD
+// FORWARD: filename.py:[[CLANG:.*]]
// FORWARD: ld.lld
// FORWARD-SAME: "--thinlto-distributor=d.exe"
-// FORWARD-SAME: "--thinlto-remote-compiler={{.*}}clang{{[^\"]*}}"
+// FORWARD-SAME: "--thinlto-remote-compiler={{.*}}[[CLANG]]"
// FORWARD-SAME: "--thinlto-distributor-arg=a1"
// FORWARD-SAME: "--thinlto-distributor-arg=a2"
// FORWARD-SAME: "--thinlto-distributor-arg=a3"
@@ -22,8 +24,8 @@
/// that a warning is issued for unused -Xthinlto-distributor options.
// RUN: %clang -flto=thin %s -### -fuse-ld=lld --target=x86_64-linux-gnu \
// RUN: -Xthinlto-distributor=a1 -Xthinlto-distributor=a2,a3 2>&1 | \
-// RUN: FileCheck %s --check-prefix=NODIST --implicit-check-not=distributor \
-// RUN: --implicit-check-not=remote-compiler
+// RUN: FileCheck %s --check-prefix=NODIST --implicit-check-not=distributor \
+// RUN: --implicit-check-not=remote-compiler
// NODIST: warning: argument unused during compilation: '-Xthinlto-distributor=a1'
// NODIST: warning: argument unused during compilation: '-Xthinlto-distributor=a2,a3'
@@ -31,10 +33,11 @@
/// Check the expected arguments are forwarded by default with only
/// --thinlto-distributor=.
+// RUN: %python %S/filename.py %clang > %t_default.log
// RUN: %clang -flto=thin %s -### -fuse-ld=lld --target=x86_64-linux-gnu \
-// RUN: -fthinlto-distributor=d.exe -Werror 2>&1 | \
-// RUN: FileCheck %s --check-prefix=DEFAULT --implicit-check-not=distributor \
-// RUN: --implicit-check-not=remote-compiler
+// RUN: -fthinlto-distributor=d.exe -Werror >>%t_default.log 2>&1
+// RUN: FileCheck %s --input-file=%t_default.log --check-prefix=DEFAULT \
+// RUN: --implicit-check-not=distributor --implicit-check-not=remote-compiler
// DEFAULT: ld.lld
// DEFAULT-SAME: "--thinlto-distributor=d.exe"
@@ -42,10 +45,11 @@
/// Check that nothing is forwarded when the compiler is not in LTO mode, and that
/// appropriate unused option warnings are issued.
+// RUN: %python %S/filename.py %clang > %t_noflto.log
// RUN: %clang %s -### -fuse-ld=lld --target=x86_64-linux-gnu \
-// RUN: -fthinlto-distributor=d.exe 2>&1 | \
-// RUN: FileCheck %s --check-prefix=NOFLTO --implicit-check-not=distributor \
-// RUN: --implicit-check-not=remote-compiler
+// RUN: -fthinlto-distributor=d.exe >>%t_noflto.log 2>&1
+// RUN: FileCheck %s --input-file=%t_noflto.log --check-prefix=NOFLTO \
+// RUN: --implicit-check-not=distributor --implicit-check-not=remote-compiler
// NOFLTO: warning: argument unused during compilation: '-fthinlto-distributor=d.exe'
// NOFLTO: ld.lld
diff --git a/clang/test/Driver/DTLTO/filename.py b/clang/test/Driver/DTLTO/filename.py
new file mode 100644
index 0000000000000..6b6d483a2fd2a
--- /dev/null
+++ b/clang/test/Driver/DTLTO/filename.py
@@ -0,0 +1,3 @@
+from pathlib import Path
+import sys
+print(f"filename.py:{Path(sys.argv[1]).name}")
diff --git a/clang/test/Driver/DTLTO/ps5-dtlto.c b/clang/test/Driver/DTLTO/ps5-dtlto.c
index eb4691da9d2fc..b52765db5b1c7 100644
--- a/clang/test/Driver/DTLTO/ps5-dtlto.c
+++ b/clang/test/Driver/DTLTO/ps5-dtlto.c
@@ -6,14 +6,16 @@
/// Check DTLTO options are forwarded to the linker.
/// Check that options are forwarded as expected with --thinlto-distributor=.
+// RUN: %python %S/filename.py %clang > %t_forward.log
// RUN: %clang -flto=thin %s -### --target=x86_64-sie-ps5 \
// RUN: -Xthinlto-distributor=a1 -Xthinlto-distributor=a2,a3 \
-// RUN: -fthinlto-distributor=d.exe -Werror 2>&1 | \
-// RUN: FileCheck %s --check-prefix=FORWARD
+// RUN: -fthinlto-distributor=d.exe -Werror >>%t_forward.log 2>&1
+// RUN: FileCheck %s --input-file=%t_forward.log --check-prefix=FORWARD
+// FORWARD: filename.py:[[CLANG:.*]]
// FORWARD: prospero-lld
// FORWARD-SAME: "--thinlto-distributor=d.exe"
-// FORWARD-SAME: "--thinlto-remote-compiler={{.*}}clang{{[^\"]*}}"
+// FORWARD-SAME: "--thinlto-remote-compiler={{.*}}[[CLANG]]"
// FORWARD-SAME: "--thinlto-distributor-arg=a1"
// FORWARD-SAME: "--thinlto-distributor-arg=a2"
// FORWARD-SAME: "--thinlto-distributor-arg=a3"
@@ -22,8 +24,8 @@
/// that a warning is issued for unused -Xthinlto-distributor options.
// RUN: %clang -flto=thin %s -### --target=x86_64-sie-ps5 \
// RUN: -Xthinlto-distributor=a1 -Xthinlto-distributor=a2,a3 2>&1 | \
-// RUN: FileCheck %s --check-prefix=NODIST --implicit-check-not=distributor \
-// RUN: --implicit-check-not=remote-compiler
+// RUN: FileCheck %s --check-prefix=NODIST --implicit-check-not=distributor \
+// RUN: --implicit-check-not=remote-compiler
// NODIST: warning: argument unused during compilation: '-Xthinlto-distributor=a1'
// NODIST: warning: argument unused during compilation: '-Xthinlto-distributor=a2,a3'
@@ -31,18 +33,21 @@
/// Check the expected arguments are forwarded by default with only
/// --thinlto-distributor=.
+// RUN: %python %S/filename.py %clang > %t_default.log
// RUN: %clang -flto=thin %s -### --target=x86_64-sie-ps5 \
-// RUN: -fthinlto-distributor=d.exe -Werror 2>&1 | \
-// RUN: FileCheck %s --check-prefix=DEFAULT --implicit-check-not=distributor \
-// RUN: --implicit-check-not=remote-compiler
+// RUN: -fthinlto-distributor=d.exe -Werror >>%t_default.log 2>&1
+// RUN: FileCheck %s --input-file=%t_default.log --check-prefix=DEFAULT \
+// RUN: --implicit-check-not=distributor --implicit-check-not=remote-compiler
+// DEFAULT: filename.py:[[CLANG:.*]]
// DEFAULT: prospero-lld
// DEFAULT-SAME: "--thinlto-distributor=d.exe"
-// DEFAULT-SAME: "--thinlto-remote-compiler={{.*}}clang{{[^\"]*}}"
+// DEFAULT-SAME: "--thinlto-remote-compiler={{.*}}[[CLANG]]"
/// Check that the arguments are forwarded unconditionally even when the
/// compiler is not in LTO mode.
+// RUN: %python %S/filename.py %clang > %t_noflto.log
// RUN: %clang %s -### --target=x86_64-sie-ps5 \
-// RUN: -fthinlto-distributor=d.exe -Werror 2>&1 | \
-// RUN: FileCheck %s --check-prefix=DEFAULT --implicit-check-not=distributor \
-// RUN: --implicit-check-not=remote-compiler
+// RUN: -fthinlto-distributor=d.exe -Werror >>%t_noflto.log 2>&1
+// RUN: FileCheck %s --input-file=%t_noflto.log --check-prefix=DEFAULT \
+// RUN: --implicit-check-not=distributor --implicit-check-not=remote-compiler
|
✅ With the latest revision this PR passed the Python code formatter. |
This appears to still break the testing. Can we please revert the original PR to get our bot back to green? |
It would involve reverting a number of commits. Can you approve #159158? @petrhosek approved the same commit (although with an incorrect description) earlier: #159050. |
) Make the test regexes more permissive to fix buildbot failures caused by the merge of PR #159129. This mirrors the earlier fix in PR #148908. We retain cross-project-test coverage to verify that the path content is correct. A follow-up will update the tests to robustly check the Clang executable filename, likely along the lines of PR #159151. Short-term unbreak to keep the buildbots green without reverting a chain of commits.
it seems that this needs rebasing. |
|
||
// DEFAULT: ld.lld | ||
// DEFAULT-SAME: "--thinlto-distributor=d.exe" | ||
// DEFAULT-SAME: "--thinlto-remote-compiler={{[^"]+}}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebasing via the GitHub GUI seems to have dropped this part of the change. We should be capturing and checking: "--thinlto-remote-compiler={{.*}}[[CLANG]]"
as in the other cases. I don't expect this to cause a build bot breakage as the code we have ended up with here is a revert to the original code that has lived upstream for several weeks previous to the latest round of changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in: #159418.
This change broke our builders again:
Looks like we need to remove the |
DTLTO is still incompatible with llvm-driver, but the test started passing after llvm#159151.
Remove XFAILs for Multicall. DTLTO is still incompatible with llvm-driver, but these tests now pass after llvm#159151. Modify a missed regex to use filename.py (missed in llvm#159151). Tighten overly greedy regexes to prevent spurious failures.
The Clang driver tests tried to match the Clang executable name via a regular expression. This failed on some buildbots where the name was not anticipated.
Make the test more robust by extracting the filename with Python and appending a line to the output. This is then captured into a FileCheck variable and used directly in the check for the executable name.
Should fix buildbot failures caused by the merge of PR #159129.